Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

macOS/iOS: Use raw-window-metal to get a CAMetalLayer from raw-window-handle #2561

Merged
merged 1 commit into from
Sep 11, 2024
Merged

macOS/iOS: Use raw-window-metal to get a CAMetalLayer from raw-window-handle #2561

merged 1 commit into from
Sep 11, 2024

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Sep 8, 2024 •
edited
Loading

  1. Update documentation to reflect any user-facing changes - in this repository.
  2. Make sure that the changes are covered by unit-tests.
    • I've done this in the raw-window-metal crate itself. Surface construction should be covered by existing examples.
  3. Run cargo clippy on the changes.
  4. Run cargo +nightly fmt on the changes.
  5. Please put changelog entries in the description of this Pull Request
  6. Describe in common words what is the purpose of this change, related GitHub Issues, and highlight important implementation aspects.

The logic for creating the CAMetalLayer if it doesn't exist is complex, and currently Vulkano does it wrong and prevents Winit from scheduling redraw events at the correct time. I have abstracted this away into raw-window-metal, see the major PR here, it works is by creating a layer, and inserting that as a sublayer, just like we did on iOS before. The bounds are then kept in-sync with an observer, ensuring smooth resizing.

This PR also fixes the compilation errors on iOS, and adds preliminary support for tvOS.

The implementation now solely uses VK_EXT_metal_surface, which was added in 2018, and I've removed support for VK_MVK_ios_surface / VK_MVK_macos_surface, which are deprecated, and only available a year and a half earlier anyhow.

See also corresponding PR to ash and PR to wgpu.

Changelog:

### Breaking changes
Changes to `Surface`:
- `Surface::update_ios_sublayer_on_resize` was removed as it is no longer necessary.
- `Surface::from_window_ref` was changed to use `VK_EXT_metal_surface` internally on macOS and iOS.

### Additions
- Add support for tvOS.

### Bugs fixed
- Make resizing smooth on macOS and iOS, and let it interoperate better with windowing libraries.
- Fix compiling on iOS.

@Rua
Copy link
Contributor

Rua commented Sep 8, 2024

I don't think support for extensions should be removed. Vulkano should work on any Vulkan-supporting device, including old ones with deprecated extensions.

@marc0246
Copy link
Contributor

marc0246 commented Sep 8, 2024

Also note that vulkano-win is deprecated. If you revert the breaking changes to Surface, you don't need to update vulkano-win. There's no point updating it.

@madsmtm
Copy link
Contributor Author

madsmtm commented Sep 8, 2024 •
edited
Loading

Thing is, the support that Vulkano previously used for these two included the somewhat implicit feature that you can pass CAMetalLayer directly. This feature was only added to the spec in 2020, see the diff. So this PR is actually only improving the support that Vulkano has for older versions.

But sure, I'll keep the extension code itself in.

@madsmtm madsmtm marked this pull request as ready for review September 9, 2024 20:43
@madsmtm
Copy link
Contributor Author

madsmtm commented Sep 9, 2024 •
edited
Loading

I've released raw-window-metal v1.0.0 now, so this is ready to be reviewed.

I guess if we wanted to keep this free from breaking changes, we could keep the update_ios_sublayer_on_resize as a no-op, and deprecate it instead?

@marc0246
Copy link
Contributor

marc0246 commented Sep 9, 2024

I think removing that function is fine. Apple support was certainly scuffed and this looks like a fine improvement which I'm grateful for. I just don't like the idea of removing support for extensions and updating vulkano-win.

@madsmtm
Copy link
Contributor Author

madsmtm commented Sep 9, 2024

About vulkano-win, I felt like if given you still have it in the git repo, then I might as well update it. I don't really care whether you end up releasing the changes or not.

@madsmtm madsmtm mentioned this pull request Sep 9, 2024
Merged
@marc0246
Copy link
Contributor

marc0246 commented Sep 9, 2024

Yes that's certainly the problem with deprecation. We have to keep releasing versions otherwise it wouldn't be deprecation. I would have preferred vulkano-win would just die already because people keep updating it despite it being deprecated, not part of the workspace, and despite this comment. The idea of deprecation is that it strictly continues to work. Anything more is a waste of time on everyone's part, and even has the potential to instroduce new bugs like every change does.

Anyway, that's enough ranting from me. The PR LGTM but neither I nor Rua have a way to test it. @hakolao would you happen to have time?

@hakolao
Copy link
Contributor

hakolao commented Sep 11, 2024 •
edited
Loading

My mac's so slow nowadays so I don't want to do much with it. I ran the triangle fine, anything else to test? @marc0246. Resizing was smooth

@marc0246
Copy link
Contributor

Thanks Okko! The examples all do the windowing the same so surely that's enough. @Rua do you have anything else to add?

@Rua
Copy link
Contributor

Rua commented Sep 11, 2024

No, looks good now. Just a merge conflict.

@madsmtm
Use raw-window-metal to get a CAMetalLayer from raw-window-handle
5e8b57a 
The way `raw-window-metal` works is by creating a layer, and inserting
that as a sublayer, just like we did on iOS before. The bounds are then
kept in-sync with an observer, ensuring smooth resizing.

This also fixes compilation errors on iOS, and adds preliminary support
for tvOS.

The implementation now solely uses `VK_EXT_metal_surface`, which was
added in 2018, instead of `VK_MVK_ios_surface` / `VK_MVK_macos_surface`,
which are deprecated, and only available a year and a half earlier
anyhow.

Note that apart from the above, there is a slight behavioral change on
macOS: we no longer set `edgeAntialiasingMask` on the layer, as it's not
really required, and allows us to avoid depending on `objc2` directly.
It was introduced without motivation in 40e0b24, so I doubt anyone uses
it, and if they do, they can change it on the layer themselves.
@marc0246
Copy link
Contributor

Thank you for blessing us with raw-window-metal @madsmtm! It looks like an awesome addition to the ecosystem ❤️

@marc0246 marc0246 merged commit 0af3fb3 into vulkano-rs:master Sep 11, 2024
5 of 6 checks passed
marc0246 added a commit that referenced this pull request Sep 11, 2024
@marc0246
#2561 changelog
a0b909f
@madsmtm madsmtm deleted the raw-window-metal branch September 11, 2024 18:10
@madsmtm madsmtm mentioned this pull request Nov 18, 2024
Open
61 tasks
@madsmtm madsmtm mentioned this pull request Nov 25, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@madsmtm @Rua @marc0246 @hakolao